-
-
Notifications
You must be signed in to change notification settings - Fork 358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add hint label to note text #5440
Conversation
Well, to be absolutely sure, I guess one could also use spaces. E.g. ( https://www.dot.at ). Looks fine, too. But, your decision. LGTM |
Another option would be |
Thanks, I think that looks better and it avoids the parentheses. |
Oh, that brings me to the next issue: It doesn't work for
I'd go for the second one if there is no objection or better idea. |
I guess there is no easy way to append the hint text after the element URL? That would be consistent and clean:
|
Maybe without the "for" but instead with a "-"? |
val leaveNoteContext = if (hintLabel.isNullOrBlank()) { | ||
"Unable to answer \"$questTitle\"" | ||
} else { | ||
"Unable to answer \"$questTitle – $hintLabel\"" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong placement of "
?
Should it really be
Unable to answer "Are these opening hours still correct? - on level 2: Mobile Service (Mobile Phone Shop)"
and not rather
Unable to answer "Are these opening hours still correct?" - on level 2: Mobile Service (Mobile Phone Shop)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really have a preference here, if you like like the second variant more I'll change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I like the second option more, or actually, the full string would be
Unable to answer "Are these opening hours still correct?" - on level 2: Mobile Service (Mobile Phone Shop) for https://osm.org/node/....
wouldn't it? Actually, my suggestion was to not use "for" at all, but use "-", i.e.
Unable to answer "Are these opening hours still correct?" - on level 2: Mobile Service (Mobile Phone Shop) - https://osm.org/node/....
(Because the "for" feels out of place there, "phone shop for element", eh?
fixes #5398
This changes the note text as discussed in #5398.
I chose to use
<
and>
for the link, as I suspect that links are parsed by different consumers (osm.org, notesReview, and probably more that I don't think of) and ideally none of them should produce broken links.Though I'm not sure how nice this is in all cases, e.g. with level:
Unable to answer "Are these opening hours still correct?" for on level 2: Mobile Service (Mobile Phone Shop) https://...